Is this the expected error for this function?
Or should the result be null?

The creator of the original https://issues.apache.org/jira/browse/CALCITE-6145 suggested to throw an exception with an exact description.

I've checked the behavior of PostgreSQL (v15):

SELECT TRIM() AS trimmed_string;

leads to a syntax error: Query Error: error: syntax error at or near ")".

SELECT TRIM("") AS trimmed_string;

leads to an error: Query Error: error: zero-length delimited identifier at or near """".

SELECT TRIM(null) AS trimmed_string;

returns null.

So I think this aligns with the behavior of PostgreSQL throwing an error during query parsing, but I think the message could be a bit better: trim cannot be called without arguments maybe?

I've added an explicit test for when the value is null, which indeed returns null (aligns with PostgreSQL's behavior).

WDYT? @mihaibudiu

If the function trim() need to have arguments, then the type checker should have complained about not finding a function with a signature without arguments. I don't expect the parser to complain about it, that is strange. The parser should just accept the function, and the type checker should validate it.

The original message is not good, since clearly there is no 'null' involved. However, if the error comes from the parser, there isn't much you can do about it.

"The second argument of trim cannot be null"
It's not clear what "string" you are referring to.

I think it's either the first or the third:

You can specify one argument (only the string to trim) or three (where to trim (leading, trailing both sides), what to trim from the string, the string to trim).

"The second argument of trim cannot be null"

This is not 100% correct I think as the string to trim from can be null: trim('a' from cast(null as varchar(1))) returns null, which is valid. I think it's more about the absence of the argument, which feels to me like a different thing than a value/field being null.

Ideally we should correctly fall through to the default case or handle the cases, where the string to trim is absent explicitly.

something is wrong here. If you reached this point you have one argument.

I think I've two (flag from which side(s) to trim, string to trim from the 3rd operand) right?

For trim(both 'a' from 'aAa') you've three operands:

operand[0]: "BOTH"
operand[1]: "a"
operand[2]: "aAa"

yes, and the message says that there are "no arguments"

Yup, currently adjusting this

Ok so I've debugged through this:

If you call trim() the following happens:

• Enters the block for 3 operands (first gets a default value, second and third are null, not in the sense of their value, but of their absence)
• operands[0] gets set to BOTH
• operands[1] gets so to " "
• operands[2] is null (indicating absence of the arg not that its actual value is null)

So the actual method call from a user perspective was one without arguments (trim()).

trim(" a ") works fine
trim(both " a ") works fine
trim("a" from) syntax error (string to trim is absent)
trim(both "a" from) syntax error (string to trim is absent)

So to handle the no-args case I think it's correct to check for if (operands[2] == null) in the block with 3 operands. I'll add a comment explaining it, otherwise it's confusing I think.

yes, the first two arguments have some default values

@mihaibudiu @timgrein Perhaps we can refer to SparkSQL's exception information.
SparkSQL:

spark-sql> select trim();
Error in query: Invalid number of arguments for function trim. Expected: one of 1 and 2; Found: 0; line 1 pos 7